Skip to content

Conversation

@innocenzi
Copy link
Member

This pull request changes the order of loaded config to put .local.config.php and .production.config.php last, so they can override their "default" counterpart.

This allows having different configurations in different environments. For instance, I may have a storage.config.php using S3 by default, but a storage.local.config.php using a filesystem for local development.

Related: #274

@innocenzi innocenzi requested a review from brendt May 23, 2025 15:37
@brendt
Copy link
Member

brendt commented May 24, 2025

I proposed environment-specific config files a long time ago (on Discord), but remember people not liking the idea. The question was asked: why not do it with env variables? Furthermore, because or config files are php files, you could have conditionals:

if (get(Environment::class)->isLocal()) {
    return new SQLiteConfig();
} 

return new PostgresConfig();

I'm not against per-file configs, but I also don't have a very strong opinion on the matter.

@innocenzi
Copy link
Member Author

Why not support both? People preferring conditionals in a single file can do this, and those preferring adding a storage.dev.config.php file to .gitignore can do that as well

That being said, Tempest should have a "recommendation" regarding the best way to handle this, because people are going to look for the idiomatic, Tempest way

@brendt
Copy link
Member

brendt commented May 25, 2025

Ok two thoughts:

  • I believe this PR prioritizes one over the other, but still loads all? If we allow this syntax, it shouldn't load non-matching environment config files at all.
  • I think the recommendation should be:
  1. Use .config.php by default
  2. Use env() when possible
  3. Use separate, environment specific config files when needed
  4. If you need even more flexibility, do whatever you want by making your config files conditional

@innocenzi
Copy link
Member Author

innocenzi commented May 25, 2025

Yes, it loads all, so the last ones would override the previous ones. I'll update the PR for only loading .local in dev and .prod in production

And I totally agree with that recommendation 👍

@brendt
Copy link
Member

brendt commented May 26, 2025

Cool. Feel free to merge then when it's been changed :) Will you update the docs as well?

@innocenzi innocenzi merged commit 0306cbd into tempestphp:main May 26, 2025
65 checks passed
@innocenzi innocenzi deleted the feat/config-order branch May 26, 2025 12:40
@innocenzi
Copy link
Member Author

Sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants